[EXPORTER] prometheus: add otel_scope_name and otel_scope_version labels #2293
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dashpole
force-pushed
the
prometheus_scope_labels
branch
2 times, most recently
from
232617f to
212071b
Compare
Codecov Report
@@ Coverage Diff @@
## main #2293 +/- ##
=======================================
Coverage 87.39% 87.39%
=======================================
Files 199 199
Lines 6009 6009
=======================================
Hits 5251 5251
Misses 758 758 |
dashpole
force-pushed
the
prometheus_scope_labels
branch
4 times, most recently
from
6f7795f to
58afc94
Compare
Any pointers on what this might be? |
dbolduc
reviewed
Sep 11, 2023
There was a problem hiding this comment.
EDIT: I was wrong about this. There is an implicit conversion to std::string. I am not sure what will happen in the case where OTel uses the standard library though.
Also, maybe related, this part of the implementation is error prone:
This assumes that the string_views are null-terminated, which might not be the case. I can send a PR to fix it.
dashpole
force-pushed
the
prometheus_scope_labels
branch
3 times, most recently
from
bdcb870 to
4fe16cf
Compare
dbolduc
reviewed
Sep 18, 2023
There was a problem hiding this comment.
If you want to see the CI pass, you could steal the changes from #2301
Basically make this
const opentelemetry::sdk::instrumentationscope::InstrumentationScope *scope_ = nullptr;
dashpole
force-pushed
the
prometheus_scope_labels
branch
3 times, most recently
from
fbfc0c4 to
49946cb
Compare
dbolduc
reviewed
Sep 21, 2023
This comment was marked as resolved.
This comment was marked as resolved.
dashpole
force-pushed
the
prometheus_scope_labels
branch
from
fdcaa01 to
1e5f6c5
Compare
dashpole
force-pushed
the
prometheus_scope_labels
branch
from
1e5f6c5 to
d9a8159
Compare
punya
reviewed
Sep 25, 2023
dashpole
force-pushed
the
prometheus_scope_labels
branch
3 times, most recently
from
c178713 to
850dfc4
Compare
|
Please merge with a recent main, to resolve the conflicts. |
dashpole
force-pushed
the
prometheus_scope_labels
branch
from
079f37a to
afa9009
Compare
|
@marcalff done |
lalitb
approved these changes
Oct 2, 2023
|
I'm happy to wait and rebase my PR. |
marcalff
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #1841
Changes
From https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/compatibility/prometheus_and_openmetrics.md#instrumentation-scope-1:
This adds the otel_scope_name and otel_scope_version labels to all metrics, if they are non-empty.